Fix presigned URL download progress tracking for range requests and add listener tests#6977
Conversation
| } | ||
|
|
||
| @Test | ||
| public void downloadFileWithPresignedUrl_success_shouldInvokeListener() throws Exception { |
There was a problem hiding this comment.
These new tests are using the mock S3CrtAsyncClient which is not a multipart client mock. I think these would pass regardless of the fix?
Are these related to the last PR? Why did we ship these separately?
There was a problem hiding this comment.
Right, the integration tests already verify the fix. But since S3TransferManagerListenerTest only uses a mock S3CrtAsyncClient removed the tests in it and created S3TransferManagerPresignedUrlListenerWiremockTest using S3AsyncClient with and without multipartEnabled to verify listener callbacks fire across all code paths.
Shipped separately from #6951 when I identified the range check fix was needed to make the multipart+range test case work correctly.
jencymaryjoseph
merged commit 63d4561
into
feature/master/pre-signed-url-getobject
|
This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one. |
2 participants
Motivation and Context
Presigned URL downloads with a user-specified range and multipart enabled produced no progress tracking output. The
wrapForNonSerialFileDownloadwrapper was applied, but when range is set, multipart is skipped (single request) and split() is never called — so the wrapper's progress hooks never fire.Modifications
GenericS3TransferManager.downloadFileWithPresignedUrl: Added range == null check — uses wrapForNonSerialFileDownload for multipart without range, falls back to wrapResponseTransformer when range is set.GenericS3TransferManager.downloadWithPresignedUrl:Same range == null check.Testing
S3TransferManagerListenerTest: Added TransferListener callback verification tests for presigned URL downloads — verifies transferInitiated, bytesTransferred, transferComplete fire on success, and transferInitiated, transferFailed fire on failure.S3TransferManagerPresignedUrlDownloadIntegrationTest: Added parameterized progress tracking tests (multipart/non-multipart × range/no-range) asserting totalBytes, transferredBytes, and downloaded size correctness.Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License